Conversation
|
|
||
| on: | ||
| pull_request_target: | ||
| pull_request: |
There was a problem hiding this comment.
I think without this, gha won't run for PRs from forks. Need to double check cc @Pearl1594 @shwstppr @DaanHoogland @vishesh92
There was a problem hiding this comment.
Pull request overview
This PR refactors the PR build/comment flow by moving PR commenting into a separate workflow_run workflow and switching the build workflow to run on pull_request rather than pull_request_target.
Changes:
- Add a new
workflow_runworkflow to comment/update PR build results after the build completes. - Change the build workflow trigger from
pull_request_targettopull_requestand simplify checkout configuration. - Remove the in-workflow PR commenting job from the build workflow.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 5 comments.
| File | Description |
|---|---|
| .github/workflows/comment-pr-build.yml | New workflow that posts/updates a PR comment based on the completed build workflow run. |
| .github/workflows/build-pr-cmk.yml | Switch build trigger to pull_request, adjust checkout, and remove the inline commenting job. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| @@ -36,10 +36,9 @@ jobs: | |||
| outcome: ${{ steps.meta.outputs.outcome }} | |||
| artifact_url: ${{ steps.meta.outputs.artifact_url }} | |||
| steps: | |||
| - name: Checkout PR HEAD | |||
| - name: Checkout PR code | |||
| uses: actions/checkout@v4 | |||
| with: | |||
| ref: ${{ github.event.pull_request.head.sha }} | |||
| persist-credentials: false | |||
There was a problem hiding this comment.
The PR title indicates the build should check out the base branch, but with on: pull_request and no explicit ref, actions/checkout will check out the PR merge ref (refs/pull/<n>/merge) by default (i.e., merged PR tip, not base). If the intention is truly to build the base branch, set ref to the base ref/SHA; otherwise the PR title/description should be updated to match the behavior.
|
❌ Build failed for PR #204. See the run: https://github.com/apache/cloudstack-cloudmonkey/actions/runs/21914741911 |
| body += `✅ Build complete for PR #${prNumber}.\n\n`; | ||
| body += `📦 Binary artifacts are available in the [workflow run](${runUrl}) (expires on ${expiryDate}).\n\n`; | ||
| body += `> **Note:** Download artifacts by clicking on the workflow run link above, then scroll to the "Artifacts" section.\n`; | ||
| body += `> _Artifacts from PR builds are for testing only and may contain unreviewed code._`; |
There was a problem hiding this comment.
| body += `> _Artifacts from PR builds are for testing only and may contain unreviewed code._`; | |
| body += `> _Artifacts from PR builds are for testing only and may contain unreviewed code at best, and malicious code at worst._`; |
There was a problem hiding this comment.
@DaanHoogland I feel that if the intention is to simply mention that binaries can contain malicious code, then we should state that directly
| body += `> _Artifacts from PR builds are for testing only and may contain unreviewed code._`; | |
| body += `> _Artifacts from PR builds are for testing only and may contain unreviewed, malicious code._`; |
6 participants
No description provided.